Skip to content

gmp: fix cross compilation to windows#132188

Closed
alexfmpe wants to merge 1 commit intoNixOS:stagingfrom
alexfmpe:cross-mingw-gmp
Closed

gmp: fix cross compilation to windows#132188
alexfmpe wants to merge 1 commit intoNixOS:stagingfrom
alexfmpe:cross-mingw-gmp

Conversation

@alexfmpe
Copy link
Member

@alexfmpe alexfmpe commented Jul 31, 2021

Things done
  • [] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

alexfmpe
alexfmpe commented Jul 31, 2021
View reviewed changes
pkgs/development/libraries/gmp/6.x.nix

# Required for "undefined reference to __memset_chk" error
# see https://github.com/msys2/MINGW-packages-dev/pull/6
NIX_LDFLAGS = lib.optionalString stdenv.targetPlatform.isWindows "-lssp";
Copy link
Member Author

@alexfmpe alexfmpe Jul 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CPPFLAGS=-fstack-protector also gets through the __memset_chk error, but later hits undefined reference to `__stack_chk_fail'`

@alexfmpe
Copy link
Member Author

alexfmpe commented Jul 31, 2021

Not sure if this should be backported to 4.3.2.nix / 5.1.x.nix

@veprbl veprbl added the 6.topic: windows Running, or buiding, packages on Windows label Jul 31, 2021
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. labels Jul 31, 2021
@ofborg ofborg bot requested review from peti and vrthra July 31, 2021 05:34
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jul 31, 2021
@Mindavi Mindavi added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jul 31, 2021
@Mindavi
Copy link
Contributor

Mindavi commented Jul 31, 2021

Diff LGTM

@alexfmpe
Copy link
Member Author

alexfmpe commented Jul 31, 2021

I just ran into the same issue when trying to build pkgsCross.mingw32.hello on master. Same fix. Maybe this should go somewhere more general?
This seems to be a somewhat recent issue, I do not run into it on nixpkgs-20.09.
cc @Ericson2314

@alexfmpe alexfmpe mentioned this pull request Jul 31, 2021
Merged
11 tasks
@alyssais
Copy link
Member

alyssais commented Jul 31, 2021

This one also needs to target staging, due to the number of rebuilds.

@alexfmpe alexfmpe changed the base branch from master to staging July 31, 2021 10:42
@alexfmpe
Copy link
Member Author

alexfmpe commented Jul 31, 2021

This one also needs to target staging, due to the number of rebuilds.

Actually, I'm confused.
I see @NixOS/ofborg tagged this as linux/darwin rebuilds, but I'd expect lib.optionalString stdenv.targetPlatform.isWindows to prevent those from being affected? And AFAICT the mingw build was broken, so maybe no actual rebuild is triggered?

@FRidh
Copy link
Member

FRidh commented Jul 31, 2021

I see @NixOS/ofborg tagged this as linux/darwin rebuilds, but I'd expect lib.optionalString stdenv.targetPlatform.isWindows to prevent those from being affected? And AFAICT the mingw build was broken, so maybe no actual rebuild is triggered?

That's because optionalString still needs to return something, so NIX_LDFLAGS is still set. Don't worry about it.

@symphorien
Copy link
Member

symphorien commented Jul 31, 2021

I just ran into the same issue when trying to build pkgsCross.mingw32.hello on master. Same fix. Maybe this should go somewhere more general?

adding -lssp could probably go in pkgs/build-support/cc-wrapper/add-hardening.sh maybe?

@alexfmpe
Copy link
Member Author

alexfmpe commented Aug 1, 2021

Turns out this isn't quite right.
While it fixes the build issue, and makes pkgsCross.mingwW64.hello run fine (via wine) on top of master, that's not the case for staging - there it still builds but we get a runtime crash from not finding dlls.

adding -lssp could probably go in pkgs/build-support/cc-wrapper/add-hardening.sh maybe?

This ended up breaking other things, including a dependency of gmp: mcfgthreads-x86_64-w64-mingw32-git, because it can't find libssp

configure:2726: $? = 1
configure:2746: checking whether the C compiler works
configure:2768: x86_64-w64-mingw32-gcc    conftest.c  >&5
/nix/store/d8j3pwvdgqpm06qsg37d0w8j0w8vcsza-x86_64-w64-mingw32-binutils-2.35.1/bin/x86_64-w64-mingw32-ld: cannot find -lssp
collect2: error: ld returned 1 exit status

It seems actually fixing this requires #38632, which in turn requires #132340, so I'm marking this as a draft for the time being.

@alexfmpe alexfmpe marked this pull request as draft August 1, 2021 18:33
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Aug 7, 2021
@sternenseemann
mpdecimal: support cross compilation to windows
7a5f5d2 
* The __memcpy_chk fix is similar to what is proposed here
  NixOS#132188

* We also need pthreads

Didn't test running, but compiles fine.
@alexfmpe alexfmpe mentioned this pull request Aug 11, 2021
Merged
11 tasks
@alexfmpe
Copy link
Member Author

alexfmpe commented Aug 21, 2021

It seems actually fixing this requires #38632, which in turn requires #132340, so I'm marking this as a draft for the time being.

pkgsCross.mingwW64.gmp builds again on staging (likely since #133479).
I don't think this PR per-se is useful anymore, as the next improvement will come from the above linked PRs.

@alexfmpe alexfmpe closed this Aug 21, 2021
@alexfmpe alexfmpe deleted the cross-mingw-gmp branch June 28, 2024 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vrthra vrthra Awaiting requested review from vrthra

@peti peti Awaiting requested review from peti

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@LnL7 LnL7 Awaiting requested review from LnL7

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@Mic92 Mic92 Awaiting requested review from Mic92

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@ttuegel ttuegel Awaiting requested review from ttuegel

@zowoq zowoq Awaiting requested review from zowoq

Assignees

No one assigned

Labels

6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: windows Running, or buiding, packages on Windows 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@alexfmpe @Mindavi @alyssais @FRidh @symphorien @veprbl